fix: typing mismatch for empty strings being treated as undefined#11287
fix: typing mismatch for empty strings being treated as undefined#11287WhatCats wants to merge 1 commit intodiscordjs:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
16f28a2 to
847f9d3
Compare
|
Personally, I think this is a step in the right direction. However, I don't think this change alone is enough, as it will potentially bring different issues. For instance, passing an empty string to To fix that, we should make further changes here. For instance, modifying the |
I think I understand so the original idea was that you would query the empty string with Discord but your saying since the endpoint is Side Note: This makes me think the REST package shouldn't allow empty strings so you don't accidentally call a different endpoint.
I did consider this but then you would still end up explicitly calling the fetch-all endpoint for empty strings which is a problem as I originally described. I'm thinking maybe it would make sense to throw a Discord like error for empty strings manually (like you would get if you passed any other non-snowflake string to the API because we can't actually pass the empty string because it would be interpreted as a different endpoint). |
We could throw an error in |
Problem
At the moment methods like
GuildMemberManager:fetchthat have an optional string parameter are doing a falsy check for if the parameter was omitted. But consider the following typescript code:According to the typings
membershould be aGuildMemberbut its actually aCollection<Snowflake, GuildMember>because the empty string is falsy (this is why we love JS).My Suggestion
Do more exact undefined/null checks.
(I might not have found every place this is an issue)